Skip to content

Update coverage configuration to use monai as source#8725

Open
Borda wants to merge 4 commits intoProject-MONAI:devfrom
Borda:config/coverage-source
Open

Update coverage configuration to use monai as source#8725
Borda wants to merge 4 commits intoProject-MONAI:devfrom
Borda:config/coverage-source

Conversation

@Borda
Copy link
Contributor

@Borda Borda commented Jan 30, 2026

Description

This pull request makes a minor configuration update to the test coverage settings. The change restricts coverage measurement to only the monai package, rather than the entire source tree.

  • Coverage configuration: Changed the source setting in setup.cfg from . (current directory) to monai, so coverage reports now only include files within the monai package.

This addresses misleading information about coverage since it combines the package and tests, see: https://app.codecov.io/gh/Project-MONAI/MONAI

image

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

@Borda
Update coverage configuration to use monai as source
ad9b950 
Signed-off-by: jirka <6035284+Borda@users.noreply.github.com>
Copilot AI review requested due to automatic review settings January 30, 2026 11:46
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 30, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

📝 Walkthrough

Walkthrough

The coverage configuration in setup.cfg was changed: under [coverage:run] the source value was updated from . to monai, and the omit entry that excluded setup.py was removed. This narrows coverage measurement to the monai package and stops explicitly excluding setup.py. Single-line change net: +1/-2.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and clearly summarizes the main change: updating coverage configuration to use 'monai' as source.
Description check ✅ Passed The description includes all required template sections with complete information about the change and relevant context.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into dev

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI reviewed Jan 30, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request updates the coverage configuration to focus measurement exclusively on the monai package, addressing misleading coverage statistics that previously combined package code with test files.

Changes:

  • Modified the source setting in the coverage configuration from . (entire directory tree) to monai (only the package), ensuring coverage reports accurately reflect library code coverage without including test files.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Borda
drop omit = setup.py
bfa40a7 
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Jirka Borovec <6035284+Borda@users.noreply.github.com>
@Borda
Copy link
Contributor Author

Borda commented Feb 12, 2026

@KumoLiu @ericspod friendly ping... 🦩

@ericspod
Copy link
Member

ericspod commented Mar 1, 2026

Hi @Borda thanks for the suggested change. I feel though that leaving tests in as something that's being covered is useful to identify when tests are missed accidentally, or when code becomes dead. We should have near perfect coverage in that directory so it does bias the results as an overall percentage, but we would inspect the monai results to see where coverage was poor manually anyway.

@Borda
Copy link
Contributor Author

Borda commented Mar 1, 2026

Hi Borda thanks for the suggested change. I feel though that leaving tests in as something that's being covered is useful to identify when tests are missed accidentally, or when code becomes dead. We should have near perfect coverage in that directory so it does bias the results as an overall percentage, but we would inspect the monai results to see where coverage was poor manually anyway.

tbh, not sure what your judgment is, so you want to keep the test folder in the report? Note that test running is the way the coverage is generated, which is why/how I found this dead code in #8724

@ericspod
Copy link
Member

ericspod commented Mar 1, 2026

Hi Borda thanks for the suggested change. I feel though that leaving tests in as something that's being covered is useful to identify when tests are missed accidentally, or when code becomes dead. We should have near perfect coverage in that directory so it does bias the results as an overall percentage, but we would inspect the monai results to see where coverage was poor manually anyway.

tbh, not sure what your judgment is, so you want to keep the test folder in the report? Note that test running is the way the coverage is generated, which is why/how I found this dead code in #8724

I would leave the test coverage in the report, yes. It did reveal that the code you flagged wasn't being used so I think that is useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Borda @ericspod